Skip to content
This repository has been archived by the owner on Dec 4, 2020. It is now read-only.

make all NMs not charmable #419

Merged
merged 2 commits into from
Mar 27, 2020
Merged

make all NMs not charmable #419

merged 2 commits into from
Mar 27, 2020

Conversation

KnowOne134
Copy link
Contributor

I affirm:

  • that I agree to Project Topaz's Limited Contributor License Agreement, as written on this date
  • that I've tested my code since the last commit in the PR, and will test after any later commits

not all NMs were being tagged on initialize for being uncharmable. this will correct that.
if you need a NM to be charmable will have to tag it with mobMod Charmable, 1 anywhere in script beside onMobInitialize

TeoTwawki
TeoTwawki reviewed Mar 13, 2020
View reviewed changes
Copy link
Contributor

@TeoTwawki TeoTwawki left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be better to have the nm check in the other place you removed it from before so that the mobmod can work even in onmobinit. It just needed correct syntax to exempt nms that had the mobmod
https://github.com/project-topaz/topaz/pull/332/files#diff-1edaebbb5ff57fedc3a066c6ae7c62fdL3829

ibm2431
ibm2431 reviewed Mar 16, 2020
View reviewed changes
Copy link
Contributor

@ibm2431 ibm2431 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you're appending NMs to where the charmable mod already is, but should this assignment be in calculateStats?

I could be wrong, but LoadMOBList might be a better candidate, as it's already attempting to set the charmable mod, and it occurs before the call to InitializeMob, so I want to say onMobInitialize should be able to overwrite the default assignment.

If I'm reading this right, instance_loader might need to be modified too.

@ibm2431 ibm2431 added reviewed Has been reviewed by at least one Staff Member and removed review ready labels Mar 16, 2020
@KnowOne134
Copy link
Contributor Author

Should I delete all this logic and move to instance loader and zone loader?

topaz/src/map/utils/mobutils.cpp

Line 564 in 8985d2a

if(PMob->m_Type & MOBTYPE_EVENT || PMob->m_Type & MOBTYPE_FISHED || PMob->m_Type & MOBTYPE_BATTLEFIELD ||

@KnowOne134
Copy link
Contributor Author

As it is I'm just adding to an already in place system

@ibm2431
Copy link
Contributor

ibm2431 commented Mar 18, 2020

I think that setting charm mod should never have been in calculateStats. The function is used to recalculate mob stats depending on the level chosen when a mob spawns. Whether or not a mob is charmable isn't really a "stat" - it never changes from spawn-to-spawn. And as you pointed out, it occurs after mobInitialize, and I feel we shouldn't need to deliberately use a different lua function to overwrite the charm mod simply because core's assignment is in the wrong place.

KnowOne134
KnowOne134 commented Mar 27, 2020
View reviewed changes
Copy link
Contributor Author

@KnowOne134 KnowOne134 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@ibm2431 ibm2431 merged commit 5991d15 into project-topaz:release Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@TeoTwawki TeoTwawki TeoTwawki left review comments

@ibm2431 ibm2431 ibm2431 approved these changes

Assignees
No one assigned
Labels
reviewed Has been reviewed by at least one Staff Member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@KnowOne134 @ibm2431 @TeoTwawki